Skip to content

Conversation

@zjffdu
Copy link
Contributor

@zjffdu zjffdu commented Oct 10, 2017

What is this PR for?

2 main changes.

  • Introduce interpreter-parent module, so that all the interpreter can reuse the plugin defined in interpreter-parent
  • Add new plugin for copying interpreter-setting.json to interpreter dir

What type of PR is it?

[ Improvement | Refactoring]

Todos

  • - Task

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@Leemoonsoo
Copy link
Member

Each release, interpreter jars are published in maven repository and bin/install-interpreter.sh lets user download and install interpreters. Is this change keeps capability to both publish and download interpreter jars?

@zjffdu
Copy link
Contributor Author

zjffdu commented Oct 11, 2017

Yes, it keeps capability. The interpreter jar is not changed. interpreter-setting.json is still packaged into interpreter jar, but copied to interpreter dir as well in this PR. And interpreter-setting.json under interpreter dir take precedence over that in interpreter jar. see https://github.com/apache/zeppelin/blob/master/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java#L263

@Leemoonsoo
Copy link
Member

I see. Could you also explain little bit about why you decided to make interpreter-parent module instead of just add copy plugin to existing common parents zeppelin-interpreter ?

This may require some additional update

@zjffdu
Copy link
Contributor Author

zjffdu commented Oct 12, 2017

@Leemoonsoo parent project must be pom type, but zeppelin-interpreter is jar. So I have to introduce module interpreter-parent. Interpreter dev guide is updated in the new commit.

@Leemoonsoo
Copy link
Member

Ah, i see. I think i have confused between parent project and dependency.

@zjffdu
Copy link
Contributor Author

zjffdu commented Oct 13, 2017

will merge if no more discussion

@asfgit asfgit closed this in 4a3057f Oct 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants